fix(Staging-Mode): Close the container if exitStagingMode throws #24775
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR improves exitStagingMode by ensuring the container is closed when errors occur during the final flush of staged changes.
- Updated flushing logic: replaced this.outbox.flush() with this.flush() for better clarity.
- Added a try-catch block in exitStagingMode to catch errors, normalize the error, invoke the container close callback, and rethrow the normalized error.
Comments suppressed due to low confidence (1)
packages/runtime/container-runtime/src/containerRuntime.ts:3458
- Consider adding an inline comment in the catch block to document why the normalized error is rethrown after invoking this.closeFn. This additional context can improve code readability and maintainability.
} catch (error) {
ChumpChief
approved these changes
Jun 6, 2025
|
Doh, gotta fix up some test expectations now. |
markfields
force-pushed
the
sm/error-handling
branch
from
f7a29f1 to
c44d872
Compare
this.flush is incompatible with batchRunner. I would rather structure orderSequentially to be able to handle staging mode using this.flush, but it's not the priority right now.
ChumpChief
approved these changes
Jun 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes AB#41363
Description
We have several asserts and throws under the
exitStagingModeAPI, we should close the container if it throws, some of them would leave us in an invalid state.